Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix arrays comparison #18

Closed

Conversation

michallaskowski
Copy link

comparator was trying to use methods of BaseTestSuite which resulted in

609:      if (value1Type = "roList" or value1Type = "roArray") and (value2Type = "roList" or value2Type = "roArray")
610:*         return m.eqArrays(value1, value2)
611:      else if value1Type = "roAssociativeArray" and value2Type = "roAssociativeArray"
612:          return m.eqAssocArrays(value1, value2)

Member function not found in BrightScript Component or interface. (runtime error &hf4) in pkg:/source/testFramework/UnitTestFramework.brs(610)

The same for associative arrays.

Here I changed baseComparator to compare method, which solves this issue. Of course it can also be done differently, but I think this is the easiest fix, and not creating any new objects will help with readability.

RokuRnD added a commit that referenced this pull request Dec 13, 2017
 - Add a way to skip tests in the BrightScript Unit Test framework
 - fix EqValues method of BaseTestSuite for arrays. #18
 - version update.
@RokuRnD
Copy link
Contributor

RokuRnD commented Dec 13, 2017

@michallaskowski Thank you for your contribution, but we want to keep the ability to provide a custom comparator function. The PR would be closed, but please check the latest version where this issue is fixed with a little bit changed implementation that you've proposed.

Once again thanks for helping us improve the framework. If it's okay, we will add your name in the changelog. We would really appreciate further PR from you! Latest version is here: https://github.com/rokudev/unit-testing-framework/releases/tag/v.1.2.2

@RokuRnD RokuRnD closed this Dec 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants